Initial implementation - #1
Conversation
Move `app.boot()` and its tests from loopback. Fix jshint warnings. Clean up unit tests - remove dependency on global variables created by loopback's test/support.js
|
test please |
|
All is well |
|
Added another commit which implements support for |
Modify loading of `appConfig` and `dataSourceConfig` to look for
the following files:
- app.json
- app.local.{js|json}
- app.{$env}.{js|json}
- datasources.json
- datasources.local.{js|json}
- datasources.{$env}.{js|json}
where $env is the value of `app.get('env')`, which usually defaults
to `process.env.NODE_ENV`.
The values in the additional files are applied to the config object,
overwritting any existing values. The new values must be value types
like String or Number; Object and Array are not supported.
Additional datasource config files cannot define new datasources,
only modify existing ones.
The commit includes refactoring of the config-loading code into
a standalone file.
|
All is well |
Support custom project layouts where model & datasource config files
are located in a different place from the app config.
Example:
# API server
server/app.config.json
server/datasources.json
# shared between server & client
models.json
models/
# isomorphic client
client/app.config.json
client/datasources.json
|
All is well |
No changes in functionality.
When a script in `models/` or `boot/` exports a function which is not
a loopback.Model constructor, the bootstrapper immediatelly calls
this exported function wit the current `app` object.
This is providing a dependency injection mechanism for boot scripts,
so that they no longer need to know where to find the `app` object.
Note: the dependency injection is optional. Existing code getting
`app` reference via `require('../app')` will continue to work.
|
All is well |
Sub-directories of `models/` and `boot/` that cannot be required (they don't have an index.js file) are silently skipped now. This enables developers to put test files into `models/test/`.
|
All is well |
|
Added more changes as required by strongloop/loopback-example-offline-sync#7. |
|
A few high level design RFC:
|
There was a problem hiding this comment.
jshint checks that all identifier are camelCase/CamelCase. Environment variables like process.env.npm_config_host are violating that rule.
The line 114 disables that check for the scope of the current method.
It sounds like you are suggesting a re-design of the config loader. I think that is probably warranted for 2.0. But I think it is out of scope for this PR. All we are doing here is migrating functionality from loopback to a module and adding two new features (loading js files and env named files). Once we migrate, we can version loopback and this new module independently and anyone can more easily adopt a 2.0 version of this module. IMO we should save larger changes for that version. |
There was a problem hiding this comment.
Is this a formal way to document anonymous functions?
There was a problem hiding this comment.
WebStorm recognises it. The syntax comes from Google Closure Compiler: https://developers.google.com/closure/compiler/docs/js-for-compiler
I checked the docs again, the name:type syntax is not mentioned for functions, but they use it for record type:
{{myNum: number, myObject}}
For functions, they have
{function(string, boolean)}
{function(this:goog.ui.Menu, string)}
Sure. I agree. |
|
I don't see anything glaring. The code LGTM. I would like to see docs before I agree to the new config approach. Theres a lot going on just being pulled from old loopback, so I'm not 100% sure what new is being added. |
Cool, I'll merge this pull request then.
What kind of docs do you have in mind?
The first commit contains the code pulled from loopback. Everything else is new stuff. |
7 participants
Move
app.boot()and its tests from loopback.Fix jshint warnings.
Clean up unit tests - remove dependency on global variables created
by loopback's test/support.js
/to @ritch please review.
I did not get rid of
loopback.autoAttach()in this pull request, as it requires more changes.loopbackis kept as a peer dependency so that werequire('loopback')works. Once we get rid ofloopback.autoAttach, we should be able to remove that peer dependency too. Possibly use runtime version check instead.